Don't think there's much point in a PR with a revert commit. If whatever problem we were encountering on ci when referencing the npm package directly is no longer happening -- which is very possible, given that back in February we were still using slugs, so the image building pipeline was entirely different -- then this fork no longer serves any purpose. We can go back to referencing the npm package from realtime and delete this fork entirely; there are no other changes.

The first step of that would be to push a realtime branch that does that and see if it passes ci.

I don't quite understand the context of where this was failing to warrant the fix. If it was a local issue this could have been caused by a cache problem.
We can never assume the details of a nested node_modules tree, if node-pre-gyp is deduped then it will not exist in node_modules. Perhaps you found it missing and thought that was the issue (where it ought to have been in the path still).

This was as part of image building on ci -- if you think about it you'll realise it couldn't have been a local problem because locally you'd already have node-pre-gyp installed (as a dependency of grpc, gc-stats, etc.), so a bug like this where having been just installed it apparently isn't yet accessible to the pulsar-client install script just wouldn't show up.

Dedup could not have been the issue, for a start because we didn't dedupe as part of slugbuilding at the time (slugs were built on jenkins by the heroku nodejs buildpack, which did not dedup), and also because even if we did, npm path resolution (used by references to binaries in npm scripts) just uses node_modules/.bin which should contain symlinks to the relevant script or binary wherever it lives in the dep tree. (Of course, it's possible that there's a bug and dedupe does not update those symlinks appropriately, leaving it in a broken state, but as mentioned we weren't deduping anyway, so it wasn't that).

Ultimately it should be an invariant that the listed dependencies of a package are installed, and the relevant binaries accessible by npm module resolution, at the time that the package's install script is run. If that isn't true because the dependency is not installed then that's a bug in npm; if it isn't true because the dependency's binaries are not accessible then that's also a bug in npm (binary resolution). Either way explicitly installing a package listed in the dependencies as part of the package's install script should be a no-op.

But anyway, first step is to see whether it is still an issue by trying a realtime ci pass which references the pulsar-client npm package.

My point is that in a nested package you can't assume realtime/node_modules/pulsar-client-node/node_modules/node-pre-gyp/bin/node-pre-gyp,js exists when it could have been deduped to realtime/node_modules/node-pre-gyp/bin/node-pre-gyp.js. You don't need to explicitly dedupe in CI for node_modules to get into this state, you just need npm to have worked out it could dedupe node-pre-gyp when the pulsar client was installed and for that to have been committed to the shrinkwrap.

This was as part of image building on ci -- if you think about it you'll realise it couldn't have been a local problem because locally you'd already have node-pre-gyp installed

My local workflow often involves removing node_modules to ensure it's in sync with the latest shrinkwrap ,so I don't think it would have been invisible to me locally.

But anyway, first step is to see whether it is still an issue by trying a realtime ci pass which references the pulsar-client npm package.

Sounds good to me, the reason I got to this PR was because I fixed it before realising that I had reverted a whole commit so thought I could just use it for the discussion.